Repro: issue #131 invalid-server WebSocket flake (DO NOT MERGE)#161
Closed
bghgary wants to merge 1 commit intoBabylonJS:mainfrom
Closed
Repro: issue #131 invalid-server WebSocket flake (DO NOT MERGE)#161bghgary wants to merge 1 commit intoBabylonJS:mainfrom
bghgary wants to merge 1 commit intoBabylonJS:mainfrom
Conversation
…S#131 DO NOT MERGE. Expands the test to 20 iterations so CI catches the ~1/3 flake rate reported in issue BabylonJS#131. If CI passes, our fixes (BabylonJS#150, BabylonJS#160) resolved the issue and we can re-enable the single test in a follow-up. If CI fails, flake persists and needs more investigation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bghgary
added a commit
to bghgary/JsRuntimeHost
that referenced
this pull request
Apr 17, 2026
The flake reported in BabylonJS#131 was resolved by BabylonJS#150 (UrlLib onError/onClose consolidation) and BabylonJS#160 (spec-compliant close/send). Verified via repro PR BabylonJS#161 which ran 20 iterations of this test across all platform/engine combos with zero failures. Fixes BabylonJS#131. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
[Posted by Copilot on behalf of @bghgary] Repro served its purpose — 20 iterations of the invalid-server test passed on every platform/engine combo in workflow run 24587856930, including Chakra where the flake was historically observed. Superseded by #162, which restores the original single test. Closing. |
bghgary
added a commit
that referenced
this pull request
Apr 17, 2026
[Created by Copilot on behalf of @bghgary] Re-enable the "should trigger error callback with invalid server" WebSocket test that was commented out in #130 due to the flake reported in #131. ## Why this is safe to re-enable Two WebSocket-related fixes landed on `main` since the flake was observed: - **#150** — pulled in UrlLib BabylonJS/UrlLib#26, which consolidates `onError`/`onClose` dispatch on Windows and Apple. Before this, the Windows and Apple implementations only fired `onError` on connection failures without a matching `onClose`, leaving consumers (and this test) in inconsistent terminal-event states. - **#160** — made the JS-layer `close()` idempotent and `send()` spec-compliant when CLOSING/CLOSED, fixing cross-state dispatch bugs. ## Verification Repro PR #161 expanded this test to 20 sequential iterations on the same branch to exercise the historical 1/3 flake rate. If the flake persisted, P(all 20 pass) ≈ 0.03%. CI on #161 ran across every platform/engine combo — Chakra/V8/JSI on UWP/Win32, Linux (gcc, clang, sanitizers, TSan), macOS (Xcode 16.4, sanitizers, TSan), iOS (Xcode 15.2, 16.4) — and **all 20 iterations passed on every job** ([workflow run 24587856930](https://github.com/BabylonJS/JsRuntimeHost/actions/runs/24587856930)). This PR restores the original single test (no loop, default mocha timeout) as it was before #130. Fixes #131. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Created by Copilot on behalf of @bghgary]
Do not merge. This is a CI repro branch for #131.
Purpose
Issue #131 reports that the "should trigger error callback with invalid server" WebSocket test fails ~1/3 of the time. The test was commented out in #130. Since filing, the following WebSocket-related changes landed on
main:onError/onClosedispatch on Windows + Apple).close()/send()spec compliance.Running the suite 30× locally on Windows + Chakra (at pre-#150 and at pre-#160 snapshots) produced 0 failures, so the flake is not reproducible on Windows locally. This PR uncomments the test and runs 20 iterations of it so CI (Linux/macOS) has a high probability of catching the flake if it still exists: P(all pass) = (2/3)^20 ≈ 0.03% if the 1/3 flake rate still holds.
Expected outcome
Not for merge
This PR adds 20 copies of the same test (spam), which is not an acceptable final state. If the repro confirms the fix, a follow-up PR will restore just the original single test.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com